Skip to content

Background steps execution engine#4476

Open
lokesh755 wants to merge 4 commits into
mainfrom
lokesh755-background-steps-engine
Open

Background steps execution engine#4476
lokesh755 wants to merge 4 commits into
mainfrom
lokesh755-background-steps-engine

Conversation

@lokesh755
Copy link
Copy Markdown
Contributor

@lokesh755 lokesh755 commented Jun 4, 2026

Summary

Adds the core background step execution engine to the runner, enabling concurrent step execution within a single job. Background steps (background: true) run asynchronously while the foreground step loop continues. Control-flow steps (wait, wait-all, cancel) coordinate synchronization and lifecycle management.

Changes

New files

  • BackgroundStepCoordinator.cs — Coordinates background step execution, waiting, cancellation, and deferred state flushing. Manages a concurrency semaphore (default max 10).
  • ControlFlowStepData.cs — Data class for wait/wait-all/cancel steps.
  • BackgroundStepsL0.cs — Unit tests for background step coordination, waiting, cancellation, and failure propagation.

Modified files

  • StepsRunner.cs — Integrates BackgroundStepCoordinator; background steps are queued without blocking the main loop; control-flow steps are dispatched directly; implicit wait-all runs before post-hooks as a safety net.
  • JobExtension.cs — Initializes control-flow steps from BackgroundStepControl pipeline type; maps logical step IDs to external IDs for UI reference; creates implicit wait-all for uncovered background steps.
  • ExecutionContext.cs — Adds background step metadata as optional parameters to CreateChild

Key design decisions

  • Runner owns concurrency — run-service remains agnostic to step execution order
  • Non-blocking semaphore — foreground steps execute immediately even when all background slots are occupied
  • Implicit wait-all — safety net before post-hooks waits for any uncovered background steps

@lokesh755 lokesh755 force-pushed the lokesh755-background-steps-engine branch from 955429e to 13594a5 Compare June 4, 2026 03:11
@lokesh755 lokesh755 force-pushed the lokesh755-thread-safety-primitives-background-stepps branch from becd2c1 to b88b481 Compare June 4, 2026 03:25
@lokesh755 lokesh755 force-pushed the lokesh755-background-steps-engine branch 2 times, most recently from 4fbc66b to 95ab648 Compare June 4, 2026 15:26
@lokesh755 lokesh755 marked this pull request as ready for review June 4, 2026 15:26
@lokesh755 lokesh755 requested a review from a team as a code owner June 4, 2026 15:26
Comment thread src/Runner.Worker/BackgroundStepCoordinator.cs Outdated
@lokesh755 lokesh755 force-pushed the lokesh755-thread-safety-primitives-background-stepps branch from b88b481 to 5007995 Compare June 4, 2026 17:52
Base automatically changed from lokesh755-thread-safety-primitives-background-stepps to main June 4, 2026 18:08
Copilot AI review requested due to automatic review settings June 4, 2026 18:27
@lokesh755 lokesh755 force-pushed the lokesh755-background-steps-engine branch from 95ab648 to 5a09827 Compare June 4, 2026 18:27
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@lokesh755 lokesh755 force-pushed the lokesh755-background-steps-engine branch from 5a09827 to 265b98d Compare June 4, 2026 18:28
Comment thread src/Runner.Worker/JobRunner.cs
Comment thread src/Runner.Worker/JobExtension.cs Outdated
Comment thread src/Runner.Worker/JobExtension.cs Outdated
@lokesh755 lokesh755 force-pushed the lokesh755-background-steps-engine branch from 265b98d to c89cec5 Compare June 4, 2026 18:37
Comment thread src/Runner.Worker/JobExtension.cs Outdated
Comment thread src/Runner.Worker/JobExtension.cs Outdated
Comment thread src/Runner.Worker/BackgroundStepContext.cs Outdated
Comment thread src/Runner.Worker/BackgroundStepContext.cs Outdated
Comment thread src/Runner.Worker/ControlFlowStepData.cs Outdated
Comment thread src/Runner.Worker/FileCommandManager.cs
Comment thread src/Runner.Worker/ExecutionContext.cs Outdated
@lokesh755 lokesh755 changed the title Background steps engine Background steps execution engine Jun 4, 2026
Comment thread src/Runner.Worker/ControlFlowStepData.cs
Comment thread src/Runner.Worker/BackgroundStepCoordinator.cs Outdated
@lokesh755 lokesh755 force-pushed the lokesh755-background-steps-engine branch 2 times, most recently from 50a2671 to 369bbe9 Compare June 4, 2026 22:14
@lokesh755 lokesh755 force-pushed the lokesh755-background-steps-engine branch from 80ebb21 to b30b777 Compare June 5, 2026 01:11
Comment thread src/Runner.Worker/StepsRunner.cs Outdated
Comment thread src/Runner.Worker/StepsRunner.cs Outdated
@lokesh755 lokesh755 force-pushed the lokesh755-background-steps-engine branch 2 times, most recently from 48753dd to 40c6e91 Compare June 5, 2026 10:01
Comment thread src/Runner.Worker/ControlFlowStepData.cs Outdated
Comment thread src/Runner.Worker/ControlFlowStepData.cs Outdated
Comment thread src/Runner.Worker/ControlFlowStepData.cs Outdated
Comment thread src/Runner.Worker/StepsRunner.cs Outdated
Comment thread src/Runner.Worker/StepsRunner.cs Outdated
Comment thread src/Runner.Worker/ExecutionContext.cs
@lokesh755 lokesh755 force-pushed the lokesh755-background-steps-engine branch from 40c6e91 to 200d050 Compare June 5, 2026 22:44
Comment thread src/Runner.Worker/BackgroundStepCoordinator.cs Outdated
Comment thread src/Runner.Worker/BackgroundStepCoordinator.cs Outdated
@lokesh755 lokesh755 force-pushed the lokesh755-background-steps-engine branch from d6aa1f6 to 975ab20 Compare June 5, 2026 23:15
@lokesh755 lokesh755 force-pushed the lokesh755-background-steps-engine branch from 975ab20 to c6e2142 Compare June 5, 2026 23:32
public void InitializeCoordinator(int maxConcurrent)
{
_backgroundSteps.Clear();
_waitedStepIds.Clear();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is _waitedStepIds track the completed steps or uncompleted steps?

Comment on lines +145 to +157
private async Task<TaskResult> WaitForStepsAsync(string[] stepIds, CancellationToken cancellationToken)
{
var ids = stepIds ?? Array.Empty<string>();
await WaitForStepTasksAsync(ids, cancellationToken);
return CompleteWaitedSteps(ids);
}

private async Task<TaskResult> WaitForAllAsync(CancellationToken cancellationToken)
{
var remaining = _backgroundSteps.Keys.Where(id => !_waitedStepIds.Contains(id)).ToList();
await WaitForStepTasksAsync(remaining, cancellationToken);
return CompleteWaitedSteps(remaining);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we still want to have 2 extra private methods? it feels like we can just inline the code to where it used?

Trace.Info($"Background step '{stepId}' queued (slot will be acquired asynchronously).");
}

private async Task ExecuteBackgroundStepCoreAsync(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can probably move all private method under the Private helpers section down.


private async Task CancelStepsAsync(string[] cancelStepIds)
{
if (cancelStepIds == null || cancelStepIds.Length == 0) return;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (cancelStepIds == null || cancelStepIds.Length == 0) return;
if (cancelStepIds == null || cancelStepIds.Length == 0)
{
return;
}


foreach (var id in cancelStepIds)
{
FlushDeferredState(id);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we call CompleteWaitedSteps() instead?

if (step.ExecutionContext.Result == TaskResult.Failed)
{
Trace.Info($"Propagating failure from background step '{step.ExecutionContext.ContextName}' to job result.");
jobContext.Result = TaskResultUtil.MergeTaskResults(jobContext.Result, TaskResult.Failed);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we defer the caller in steprunner to update jobcontext.result?

/// </summary>
public async Task RunControlFlowAsync(IExecutionContext stepContext, object data)
{
var controlFlow = data as ControlFlowStepData;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to print anything to the output, so customer can know what we are doing under the step?

}
catch (TimeoutException)
{
Trace.Info($"Some background steps did not terminate within {graceSeconds}s grace period.");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we mark all steps that haven't finish as canceled?

{
FlushDeferredState(id);
_waitedStepIds.Add(id);
if (_backgroundSteps.TryGetValue(id, out var entry) && entry.Step.ExecutionContext.Result == TaskResult.Failed)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we only care about TaskResult.Failed? what about canceled, etc.

/// Pure data for control-flow steps (wait, wait-all, cancel).
/// Type uses Pipelines.BackgroundControlTypes string constants.
/// </summary>
public sealed class ControlFlowStepData
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we rename this BackgroundStepControlFlowData?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants